Just out of curiosity, why make syscall errors exempt?

My understanding is syscall not happen in VM realm. @Lichtso @ptaffet-jump please correct me if I'm wrong.

Syscall errors are never thrown in the middle of basic blocks, so we can easily know exactly how many CUs were consumed when the error was thrown. This is why we can safely make them exempt.

In the usual definition of basic blocks, call instructions can occur anywhere in a basic block (apart from the last instruction). So I would say this is unclear.

Also, it does not explain the need for the exemption. I would expect all CUs to be all consumed if a program failed, including if that failure was in a syscall.

I would expect all CUs to be all consumed if a program failed, including if that failure was in a syscall.

Why would you expect that? If a program makes a CPI and that invoked program returns an error, we immediately stop executing the transaction and therefore have more time to execute other transactions in a block. If we consume all remaining CUs for that failure, we are wasting that capacity for execution.

@seanyoung: This is mostly about how easy / hard it is to pinpoint the precise fault location. During syscalls the VM has stopped anyway and the return address is known on the stack. Otherwise it can be any location since the last metering (which happens at controlflow instructions). Also syscall errors are far more common and expected to happen regularly, whereas other faults such as access violation only happen in broken programs.

@Lichtso that makes sense 👍

I see that with direct mapping enabled, we convert EbpfError::AccessViolation into the SyscallError variant here: https://github.com/anza-xyz/agave/blob/af0ed22174999cb62579a0621f6b274c85ebf267/programs/bpf_loader/src/lib.rs#L1501

I am assuming that this access violation should also deplete compute units and should not be considered as an exception right?

@Lichtso @alessandrod

Yes, same reasoning as: #182 (comment)

Should consume all cu's or not?

Yes, we should. This is not caused by a syscall so the exact instruction counter would have to be calculated otherwise.

Just want to call out that this type of access violation is probably going to be fairly common when direct mapping is enabled. So it's not exactly true to say that these types of errors are "rare, exceptional situations" or "A rare case" as we say in the SIMD. We should update the SIMD to say that these types of failures are "less common" rather than rare at least.

e8c74ac

I see that with direct mapping enabled, we convert EbpfError::AccessViolation into the SyscallError variant here: https://github.com/anza-xyz/agave/blob/af0ed22174999cb62579a0621f6b274c85ebf267/programs/bpf_loader/src/lib.rs#L1501

I am assuming that this access violation should also deplete compute units and should not be considered as an exception right?

@tao-stones this should probably be made explicit in the SIMD. Maybe we can say that all EBPF errors from the vm are treated as irregular.

Mmm, imo, SIMD's text If VM execution returns any error except SyscallError, .... is clearer.

I don't think that a spec should be based on "arbitrary" rust types.
We should clarify what's the expected behavior, for example if there's a memory issue inside a syscall, is that a Syscall or Epbf error?

I agree that if we enter a syscall and that fails, we know the exact number of CUs consumed, and so there's no need to deplete them all. I'm not sure if checking the SyscallError type is the correct implementation though. (In the example above, imo a memory issue inside a syscall should NOT deplete, however I don't know if the error thrown is SyscallError or EbpfError)

I see that with direct mapping enabled, we convert EbpfError::AccessViolation into the SyscallError variant here: https://github.com/anza-xyz/agave/blob/af0ed22174999cb62579a0621f6b274c85ebf267/programs/bpf_loader/src/lib.rs#L1501

I am assuming that this access violation should also deplete compute units and should not be considered as an exception right?

@tao-stones this should probably be made explicit in the SIMD. Maybe we can say that all EBPF errors from the vm are treated as irregular.